Skip to content

fix: harden IBKR precheck runtime dispatch#353

Merged
Pigbibi merged 6 commits into
mainfrom
fix/runtime-guard-ibkr-precheck-timeout
Jul 20, 2026
Merged

fix: harden IBKR precheck runtime dispatch#353
Pigbibi merged 6 commits into
mainfrom
fix/runtime-guard-ibkr-precheck-timeout

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • parse IBKR_MONITOR_DISPATCH_TARGETS_JSON in the IBKR adapter so configured monitor targets are no longer silently dropped
  • dispatch the host service's precheck in-process to avoid self-HTTP deadlock on the current maxScale=1, single-worker deployment; keep remote targets concurrent
  • scope monitor-dispatch configuration to prechecks because dedicated Scheduler jobs already own /health warmups and /run execution
  • retire the legacy ibkr-<account>-pre-market-dry-run scheduler during runtime reconciliation
  • enforce a configurable 105-second application deadline for /dry-run, persist a structured timeout report, and return 503 before Cloud Run/Gunicorn's 300-second hard kill
  • propagate monitor target failures as a 502 instead of returning a misleading 200

Incident evidence

Runtime Guard run 29754824157 found a request to /dry-run at 2026-07-20T13:45:15Z that ran for 299.978s, returned 504, and was followed by Gunicorn WORKER TIMEOUT/SIGKILL. The caller was the legacy ibkr-u15998061-pre-market-dry-run Scheduler job. The deployed dispatcher configuration contains five targets, but application logs reported monitor_targets_count=0 because the shared parser does not recognize the IBKR-specific env name.

Verification

  • ruff check --exclude external .
  • 17 focused dispatcher/request/deadline/reconciliation tests
  • 12 Runtime Guard and scheduler deadline contract tests
  • bash tests/test_sync_cloud_run_env_workflow.sh
  • deployed configuration replay resolves 5 targets (previous runtime behavior resolved 0)

The repository's broader non-blocking test command still has five unrelated existing failures around qsl bundle metadata, dependency/strategy registry drift, and a CI text contract; 263 tests passed and 5 were skipped.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

Merge allowed: 1 finding(s) reported but none are blocking

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in application/runtime_deadline.py

recycle_current_process_after_response() does not actually wait for the worker to become unavailable before killing it; it just sleeps for one second and then sends SIGTERM to the current PID. On the current /dry-run timeout path introduced by this PR, the handler returns 503 first and only then schedules this delayed kill, so the same Gunicorn worker can accept a new /probe, /monitor-dispatch, or user request during that 1-second window and have it torn down mid-flight when the background thread fires. (line 62)

Suggestion: Recycle the worker through a post-response mechanism that stops it from taking new work first (for example a Gunicorn/worker-exit hook or immediate process exit after the response is flushed), instead of using a delayed daemon thread that sends SIGTERM to a still-accepting worker.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed both reliability findings in 8c848f3: the deadline now wraps the full /dry-run lifecycle, uses a BaseException sentinel so strategy/broker except Exception handlers cannot swallow it, skips potentially blocking persistence after interruption, emits a minimal structured timeout event, and schedules SIGTERM so Gunicorn replaces the interrupted worker. The same bounded path is used for in-process host prechecks. Focused regression suite: 31 passed; full Ruff and deployment workflow contract checks pass.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c848f3736

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread application/monitor_dispatcher.py Outdated
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the second-round findings in 8146bac. Nested in-process prechecks now only return worker_recycle_required; /monitor-dispatch waits for remote aggregation, logs the combined result, and only then schedules worker recycle. Direct /dry-run still schedules recycle at its own response boundary. Timeout reports are finalized and persisted under a separate 5-second grace deadline before the interrupted worker is retired. Added regressions for deferred nested recycle and exactly-once timeout report persistence; focused suite is now 33 passed.

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the third-round findings in 155aef4. Strategy/plugin processing keeps its 105-second deadline, while report persistence now runs afterward under an independent 5-second grace deadline; persistence timeout is logged but cannot flip a completed strategy response. Timeout reports use the same bounded persistence helper. Monitor target parsing now rejects any non-object array element with its index instead of silently dropping it. Focused suite: 35 passed; Ruff and workflow contract checks pass.

@Pigbibi
Pigbibi enabled auto-merge (squash) July 20, 2026 21:36
Co-Authored-By: Codex <noreply@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0dd4095de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread main.py
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi
Pigbibi merged commit 9d96bad into main Jul 20, 2026
5 checks passed
@Pigbibi
Pigbibi deleted the fix/runtime-guard-ibkr-precheck-timeout branch July 20, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant